Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docker-compose: Bugfix: #1598

Closed
wants to merge 3 commits into from
Closed

docker-compose: Bugfix: #1598

wants to merge 3 commits into from

Conversation

kennychennetman
Copy link
Contributor

  • fix status determination for more than one docker-compose instance

- fix status determination for more than one docker-compose instance
@knet-ci-bot
Copy link

Can one of the admins verify this patch?

# Version: 1.1.2
# Date: 2020-06-24

# Version: 1.1.1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should increase the version nr, not decrease it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

  - add determination for docker daemon status
  - improved accuracy for multiple instance
@@ -36,7 +36,7 @@
. ${OCF_FUNCTIONS_DIR}/ocf-shellfuncs

# Defaults
OCF_RESKEY_binpath_default=/usr/bin/docker-compose
OCF_RESKEY_binpath_default=/usr/local/bin/docker-compose
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldnt change the default path for it. The setting is there so you can set what the path is on your system when it's not in the default location.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to change the default path aligned to Ubuntu distribution.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to use get_release_id() for that (see ocf-distro file for logic).

cd $DIR
$COMMAND up -d || {
ocf_log err "Error. docker-compose returned error $?."
cd /tmp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldnt need to run cd /tmp in the agent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@@ -185,19 +196,25 @@ docker_compose_status()

docker_compose_start()
{

sleep 5
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldnt need to sleep here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, agree. I am still working on it... it could be improved in the future version.

docker_compose_validate_all
docker_compose_status >/dev/null 2>&1
retVal=$?
# return success if docker service is running
[ $retVal -eq $OCF_SUCCESS ] && exit $OCF_SUCCESS

sleep 10
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you need to wait here you need to add a loop checking every 1-2s (and fails when Pacemaker timeouts) to avoid waiting too much in general, and also work for cases where it takes more than 10s.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, agree. I am still working on it... it could be improved in the future version.

@kennychennetman
Copy link
Contributor Author

re-commit via #1694

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants